rustdoc: Don't try to load source files from external crates #70828
Conversation
| @@ -0,0 +1,15 @@ | |||
| // compile-flags:--remap-path-prefix={{src-base}}=/does-not-exist | |||
There was a problem hiding this comment.
Surprised {{src-base}} exists, based on the normalization rules I would've expected it to be $SRC_DIR.
| @@ -1937,6 +1937,7 @@ impl Clean<Span> for rustc_span::Span { | |||
| let hi = sm.lookup_char_pos(self.hi()); | |||
| Span { | |||
| filename, | |||
| cnum: lo.file.cnum, | |||
There was a problem hiding this comment.
Heh this is really simple. Although, perhaps a better thing to do would be to keep lo.file? And remove the span_to_filename call, which is then wasteful.
Also cc @Aaron1011 I hope this will continue to work.
There was a problem hiding this comment.
That's actually what I originally tried but it ended up quite a bit more complicated than this. The main advantage to keeping the full SourceFile is that rustdoc won't have to load the source files from disk again. I plan to do that as a follow up.
There was a problem hiding this comment.
@eddyb: This looks good to me: were you thinking of imported spans/hygiene?
There was a problem hiding this comment.
Mostly knowing the CrateNum a file came from.
| @@ -0,0 +1,15 @@ | |||
| // compile-flags:--remap-path-prefix={{src-base}}=/does-not-exist | |||
|
|
|||
| #![doc(html_root_url = "https://example.com/")] | |||
There was a problem hiding this comment.
Can you add a test that uses thread_local! from libstd and checks that the resulting docs have doc.rust-lang.org URLs? Since that was my smallest example for #70757 (but I guess I left it at the end of this comment: #70025 (comment)).
The libstd URLs should be already valid today, so it'd be a good test.
There was a problem hiding this comment.
I've now added a test using thread_local!.
| // skip non-local items | ||
| && item.def_id.is_local() | ||
| // skip non-local files | ||
| && item.source.cnum == LOCAL_CRATE | ||
| { | ||
| // If it turns out that we couldn't read this file, then we probably | ||
| // can't read any of the files (generating html output from json or |
There was a problem hiding this comment.
Do you want to close #70757 without fixing the thing where include_sources would be set to false? I guess it might be hard to actually support only being able to emit some of the files, and is much less relevant now.
Feel free to leave a FIXME comment and open an issue about it.
There was a problem hiding this comment.
Yeah, I guess we can leave #70757 open to track removing this error case.
Local items defined in external macros shouldn't generate rendered source files and should link to the external crate's docs instead.
ollie27
force-pushed
the
rustdoc_external_macro_src
branch
from
6e4aa34 to
6f96dc2
Compare
|
@bors r+ Thanks! |
|
📌 Commit 6f96dc2 has been approved by |
bors
added
the
S-waiting-on-bors
Rollup of 7 pull requests Successful merges: - rust-lang#70134 (add basic support of OsStrExt for HermitCore) - rust-lang#70565 (Add inline attributes for functions used in the query system) - rust-lang#70828 (rustdoc: Don't try to load source files from external crates) - rust-lang#70870 (Fix abuses of tykind::err) - rust-lang#70906 (Suggest move for closures and async blocks in more cases.) - rust-lang#70912 (Do not suggest adding type param when `use` is already suggested) - rust-lang#70930 (add tracking issue to `VecDeque::make_contiguous`) Failed merges: r? @ghost
jyn514
added
the
T-rustdoc
Local items defined in external macros shouldn't generate rendered source files and should link to the external crate's docs instead.
Part of #70757
r? @GuillaumeGomez
cc @eddyb